gh-119127: Fix _functools.Placeholder singleton#124601
Conversation
* The module state now stores a strong reference to the Placeholder singleton. * Use a regular dealloc function. * Add Py_TPFLAGS_HAVE_GC flag and traverse function to help the GC to collect the type when _functools extension is unloaded.
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
This looks like the right fix. I haven't manually confirmed that this fixes the refleak, but this is the proper way to implement singletons. (Originally, it was just an immortal object that never gets freed, and therefore causes a leak -- it gets worse as new subinterpreters are created and the module is reset.)
Thank you, Victor! Should this be targeting gh-124586 rather than gh-119127?
I checked manually that test_ast no longer leaks with this change. |
|
!buildbot refleaks |
|
🤖 New build scheduled with the buildbot fleet by @Eclips4 for commit 37e994a 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
Eclips4
left a comment
Eclips4
left a comment
There was a problem hiding this comment.
LGTM. I've checked it locally and there's no leak in test_ast with this patch applied.
I run buildbots to make sure that everything is fine.
| _Py_SetImmortal(placeholder); | ||
| PyObject_GC_UnTrack(self); | ||
| PyTypeObject *tp = Py_TYPE(self); | ||
| tp->tp_free((PyObject*)self); |
There was a problem hiding this comment.
| tp->tp_free((PyObject*)self); | |
| tp->tp_free(self); |
nit: there's no need to casting it to PyObject * since it's already a PyObject *
|
I will wait for a few Refleaks buildbots to complete before merging this change. |
|
Four Refleaks workers completed: test_datetime still leaks, but that's unrelated to this fix. test_ast no longer leaks on these workers. I consider that the fix works as expected and merge my PR. |
See #124606 for test_datetime leak. |
6 participants
Uh oh!
There was an error while loading. Please reload this page.